CSFLE auto encryption tests improvements #1788
Conversation
Added support for schema 1.23 JAVA-5792
- Added scala UnifiedTest support. - Added scala unified tests for ClientEncryption and Crud JAVA-5674
rozza
requested review from
nhachicha and
vbabanin
and removed request for
a team and
nhachicha
| String collectionName = arguments.getString("collection").getValue(); | ||
|
|
There was a problem hiding this comment.
[nit] We could simplify this with:
String collectionName = arguments.remove("collection").asString().getValue();
Since we already follow the practice of removing consumed parameters/options, this keeps the approach consistent. For example, see UnifiedCrudHelper.java#L1806. The parsing is scoped to a single method (BsonDocument per operation), so we don’t risk cross-contamination. This also removes the need for the case "collection": break.
driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedClientEncryptionHelper.java
Outdated
Show resolved
Hide resolved
| @@ -188,7 +188,7 @@ case class SyncMongoDatabase(wrapped: MongoDatabase) extends JMongoDatabase { | |||
| viewOn: String, | |||
| pipeline: java.util.List[_ <: Bson] | |||
| ): Unit = { | |||
| throw new UnsupportedOperationException | |||
| wrapped.createView(unwrap(clientSession), viewName, viewOn, pipeline.asScala.toList).toFuture().get() | |||
There was a problem hiding this comment.
[nit] Could we match the formatting used below for consistency, unless this is from Spotless auto-formatting?
There was a problem hiding this comment.
Yes thats a Spotless automatic change - occurs once the line reaches a certain limit.
There was a problem hiding this comment.
Given that the legacy CSFLE tests under the /legacy folder in the specification repository have been converted to the unified format under the /unified tests folder in this PR, do we still need AbstractClientSideEncryptionTest that runs the legacy tests? Or are there still some tests that haven’t yet been converted to the unified format?
By a quick check, looks like all file names in legacy folder are present in unified folder.
| @@ -110,7 +110,7 @@ public abstract class UnifiedTest { | |||
| private static final Set<String> PRESTART_POOL_ASYNC_WORK_MANAGER_FILE_DESCRIPTIONS = Collections.singleton( | |||
| "wait queue timeout errors include details about checked out connections"); | |||
|
|
|||
| private static final String MAX_SUPPORTED_SCHEMA_VERSION = "1.22"; | |||
There was a problem hiding this comment.
I see that schemaVersion has been increased to 1.25 in the unified encryption tests. Should we also bump it to 1.25?
There was a problem hiding this comment.
Not yet, that work is coming in the next PR (JAVA-5851)
…fiedClientEncryptionHelper.java Co-authored-by: Viacheslav Babanin <[email protected]>
No description provided.